Summary

Solid, well-scoped fix for the NPU embedder thrash (#1744): moving the NPU profile onto the FLM-native embed-gemma-300m-FLM so chat and embeddings stay co-resident keeps the whole feature on one backend, and making the embedder a property of DeviceConfig gives it a clean single source of truth. The standout design call is correctly distinguishing dimension from vector space — deriving the FAISS dim from the live embedder and invalidating stored vectors on an embedder-id change rather than a dim check. Test coverage is strong and the #1655 "no recipe" guard is explicitly asserted.

One thing the author should know before merge: the embedder-change invalidation protects every swap except the first one — an existing NPU user upgrading onto the FLM embedder has nomic vectors but no marker yet, so the swap goes undetected. That's the exact silent-corruption case this PR is built to prevent, just at the migration boundary. Details below.

Issues Found

🟡 Important — First migration onto the FLM embedder isn't invalidated (memory.py:423)

The marker is a new sidecar file, so any pre-existing memory.db has none. On the first run after upgrade get_embedder_id() returns None, the prior is not None guard short-circuits, and no clear happens — yet the marker is then stamped with the new model. For an existing NPU user this means their nomic 768-dim vectors survive and are now queried with FLM-space vectors. Same dim (768), so the vec.shape[0] != self._embedding_dim guard won't catch it either — silent similarity corruption, which is precisely the failure the design notes call out.

Treat a missing marker on a DB that already holds vectors as "unknown prior → re-embed". Truly fresh DBs (no embeddings) still skip the clear:

            prior = self._memory_store.get_embedder_id()
            # A missing marker on a DB that already holds vectors means we
            # can't prove they came from the active embedder (e.g. the first
            # upgrade onto the FLM embedder, where old nomic vectors share the
            # 768 dim). Treat unknown-prior-with-vectors as a change and
            # re-embed rather than risk a silent vector-space mismatch (#1744).
            if prior is not None:
                embedder_changed = prior != self._embedding_model
            else:
                embedder_changed = (
                    self._memory_store.get_embedding_coverage()["with_embedding"] > 0
                )
            if embedder_changed:
                cleared = self._memory_store.clear_all_embeddings()
                logger.warning(
                    "[MemoryMixin] embedder changed (%s -> %s); cleared %d stored "
                    "embedding(s) for re-embedding",
                    prior,
                    self._embedding_model,
                    cleared,
                )
            self._memory_store.set_embedder_id(self._embedding_model)

🟢 Minor — Embedder marker is a filesystem sidecar, not DB-resident (memory_store.py:221)

The marker lives in <db>.embedder while the vectors it describes live in the SQLite DB. Copying/moving memory.db without the sidecar silently resets the marker to "unknown" (→ a re-embed, harmless) and the write/clear aren't atomic with each other. A one-row settings/meta table in the same DB would travel with the data and tie the marker to the embeddings in one transaction. Not blocking — the sidecar works — but worth a thought if you add more DB-level metadata later.

🟢 Minor — Resolver fallback is a bit indirect (registry.py:303)

return DeviceConfig.__dataclass_fields__["embedding_model"].default reads as introspection magic at a glance. It does correctly pin to the single source, so this is purely a readability nit — a short comment (# nomic default, same as DeviceConfig) would save the next reader a double-take.

Strengths

• Right abstraction for the bug. Keying invalidation on embedder id + dynamically deriving the dim (rather than a dim check) is the correct fix — a dim-only guard would have missed the 768→768 nomic→FLM swap entirely, and the code comments make that reasoning explicit.
• DeviceConfig.embedding_model as single source of truth. Co-locating the embedder with the chat model/recipe/backend, fed through one resolver, is clean and matches the existing device-config pattern.
• Test coverage maps to the risks. test_npu_flm_embedder.py exercises the resolver, the MODELS entry, the init manifest, dynamic-dim, and the same-dim/different-model invalidation — including a clear.assert_not_called() for the no-change path. The test_init_command update asserts both FLM models pull by name with no recipe (gaia init --profile npu fails with status 400 (missing user. prefix for model name) #1655 guard intact).

Verdict

Approve with suggestions. No security or breaking-change concerns, and the hardware-only validation items are honestly disclosed as pending. The one Important item — the first-migration invalidation gap — is narrow (existing NPU user with pre-populated memory) but lands on the exact silent-corruption axis this PR targets, so I'd recommend handling the unknown-prior case before merge. The two minors are optional.

🟡 The new NPU embedder workflow pins actions/checkout@v6, but every other workflow in the repo (51 references across all .github/workflows/ files) uses actions/checkout@v7. If v6 doesn't exist as a valid major-version tag, the workflow will fail at the checkout step on the self-hosted NPU runner — the one environment where this test actually matters.

- uses: actions/checkout@v6   # ← should be @v7 to match every other workflow

🔴 The new CI workflow will fail on every run — it invokes pytest tests/test_npu_flm_embedder_hw.py but that path doesn't exist; the actual file added in this PR is tests/test_gemma_embedder_hw.py. The PowerShell step treats a non-zero exit as a failure (throw "NPU FLM embedder tests failed"), so the job will always error out on the NPU runner. The paths: trigger also lists the wrong filename, so changes to the real test file won't re-trigger CI either.

Fix: rename the file to tests/test_npu_flm_embedder_hw.py, or update the workflow's pytest command and paths: entries to match tests/test_gemma_embedder_hw.py. The file's own docstring (Run on NPU hardware with: python -m pytest tests/test_npu_flm_embedder_hw.py) has the same stale name and should be corrected at the same time.

python -m pytest tests/test_npu_flm_embedder_hw.py -v -rs --tb=short
if ($LASTEXITCODE -ne 0) { throw "NPU FLM embedder tests failed" }

- 'tests/test_npu_flm_embedder_hw.py'

Run on NPU hardware with:
    python -m pytest tests/test_npu_flm_embedder_hw.py -v -rs